Choice of artefacts language - #475
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Localize the report subtitle
When artefacts_language is set to a non-English language, the main title comes from ReportTranslationManager but this subtitle still hard-codes the English word for, so every localized PDF starts with a mixed-language header such as a Russian title followed by for <repo>. Route this literal through the same translation table so the first page is consistently localized.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return self.get("no") | ||
| elif key == YesNoPartial.PARTIAL: | ||
| return self.get("partial") | ||
| return None |
There was a problem hiding this comment.
When the model returns Unknown for a YesNoPartial field, or when TextGenerator.make_request() falls back to the default RepositoryReport after a parse failure, this fallback returns None. The report renderer then prints None for README/test checks that are actually unknown; before this change those enum values were rendered as Unknown, so the generated PDF now misreports indeterminate analysis results instead of preserving that state.
Useful? React with 👍 / 👎.
| - Check for the presence of the directory with tests | ||
| - Prioritize 3 most critical recommendations | ||
|
|
||
| - Write text in values on {target_language} |
There was a problem hiding this comment.
For non-English reports this blanket instruction applies to every JSON value, including structure.missing_files, which is later joined directly into the PDF. That can cause the model to translate concrete repository paths or filenames such as tests/requirements.txt, so the report can name files that do not actually exist instead of preserving the missing artifact names verbatim. Add an explicit exception for file, directory, package, and operation identifiers.
Useful? React with 👍 / 👎.
DRMPN
left a comment
DRMPN
left a comment
There was a problem hiding this comment.
- Нужно обновить документацию
docs. - Посмотреть замечания кодекса, вроде по делу написал.
- Нам обязательно нужны эти шрифты в репозитории? Почему именно эти? Без них работать не будет? Стандартные чем-то не подходят? Может их в runtime подгружать с cdn какого-нибудь?
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de26b53613
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Store normalized result for a task, keyed by its human‑readable name.""" | ||
| display_name = self._format_task_name(task) | ||
| self.results[display_name] = self._normalize_result(result) | ||
| self.results[task] = self._normalize_result(result) |
There was a problem hiding this comment.
When --scorecard is enabled, _run_plan_operation now stores the report result under the raw task key "report", but WhatHasBeenDoneReportGenerator still reads self.task_results.get("Report") before building the before/after Scorecard section. In that scenario before_scorecard is always None, so the work summary silently falls back to a single after-score table instead of the intended comparison; either keep the display key here or update the downstream lookup to "report".
Useful? React with 👍 / 👎.
| sourcerank = SourceRank(config_manager) | ||
| scheduler = ModeScheduler(config_manager, sourcerank, args, workflow_manager, git_agent.metadata) | ||
| plan = scheduler.plan | ||
| artefacts_language = plan.get("artefacts_language") |
There was a problem hiding this comment.
When the language is supplied through --config-file instead of the CLI flag, this still reads from the scheduler plan built from args; those argparse defaults were loaded from the bundled config before the custom TOML is parsed, and ConfigManager never copies [general].artefacts_language back into the plan. A run with a custom config setting artefacts_language = "Russian" will therefore keep using the bundled default English, so the new config option only works as a command-line flag unless this value is read from the loaded configuration or merged into the plan.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Сейчас потестировал, эта проблема касается всех полей которые можно изменять и там и там, например translate_readme тоже затирается
| - Check for the presence of the directory with tests | ||
| - Prioritize 3 most critical recommendations | ||
|
|
||
| - Write text in values on {target_language}, except names of files, directories and packages and operation identifiers |
There was a problem hiding this comment.
For a non-English report, this new rule tells the model to translate JSON values, but the readme Yes/No/Partial fields and documentation.tests_present are still validated against the English YesNoPartial enum. If the model follows the language instruction and emits localized values such as Да/Нет, TextGenerator.make_request() catches the validation failure and falls back to an all-Unknown report, so the prompt should explicitly exempt these constrained enum values from translation.
Useful? React with 👍 / 👎.
| pdfmetrics.registerFont(TTFont("notosanssc-Bold", os.path.join(assets_dir, "notosans-sc-bold.ttf"))) | ||
| pdfmetrics.registerFont(TTFont("notosanssc-Black", os.path.join(assets_dir, "notosans-sc-black.ttf"))) | ||
|
|
||
| # 2. Регистрируем для него отдельное семейство (чтобы ReportLab не выдавал ошибок при попытке применить теги) |
There was a problem hiding this comment.
Почему часть док стрингов была удалена?
| TRANSLATIONS = { | ||
| "English": { |
There was a problem hiding this comment.
Нам обязательно для каждого языка и модуля заполнять эти значения?
Если добавится поддержка китайского, то нужно будет ключ - значение и для него прописывать?
Я думаю может это ЛЛМке на откуп отдать?
Если у нас появятся другие значения, то как их добавлять? Может в конфиг или еще куда-то это вынести, а не в константой?
3 participants
#459
Реализован перевод отчётов на другие языки. Для основных языков можно задать все строки для перевода, остальные будут иметь, некоторый текст на английском.
Русский
Китайский